Add Trunk resource controller implementation #581

aldokimi · 2025-11-22T17:43:35Z

Summary

Adds complete Trunk resource controller implementation with subport implementation (which is necessary for the Trunk type), dependency handling, and comprehensive test coverage.

Key Features

Full CRUD operations for OpenStack trunks
Subport reconciliation (add/remove/update)
Dependency management (ports, projects)
Resource adoption and import support
Comprehensive test coverage

Testing

Unit tests for all actuator methods
Test coverage 35.7%
All tests passing

E2E Testing

-All resources are created in the K8s side
-All resources are created in the Openstack side
-All CRUD functionalities are passing the tests

Checklist

aldokimi · 2025-11-22T17:47:19Z

This fixes #364 which is mentioned as well in #577

winiciusallan

Hi, @aldokimi! I've left a few comments in the API types. Thanks for submitting this pull request, it will be very important in achieving parity with CAPO.

api/v1alpha1/trunk_type.go

winiciusallan · 2025-11-25T19:26:51Z

api/v1alpha1/trunk_type.go

+type Subport struct {
+	// portRef is a reference to the ORC Port which will be used as a subport.
+	// +required
+	PortRef KubernetesNameRef `json:"portRef"`


Suggested change

PortRef KubernetesNameRef `json:"portRef"`

PortRef KubernetesNameRef `json:"portRef,omitempty"`

winiciusallan · 2025-11-25T19:54:30Z

api/v1alpha1/trunk_type.go

+	PortID string `json:"portID,omitempty"`
+
+	// segmentationType is the type of segmentation used.
+	// +kubebuilder:validation:MaxLength=1024


I believe this should match the same validation in spec for this field.

It's OK if it's higher in the Status than in the Spec (the opposite is not OK).

api/v1alpha1/trunk_type.go

mandre

I suspect you haven't used the scaffolding tool. If it's not too much trouble, would it be possible for you to re-work your controller to use it? It ensures consistency with the other controllers and makes it much easier for reviewers to see your modifications, since there's typically a lot of boilerplate code in PRs for new controllers. See #568 (comment) for the rational. I'm currently working on updating the developers doc to make it more obvious.

I bet it was a lot of work to contribute this controller without the help of the scaffolding tool, so if that turns out to be too complicated to rework your change to make use of it, I'm ready to make an exception.

FYI, here's how I expect us to run the scaffolding tool, based on what the resource supports:

go run ./cmd/scaffold-controller \
        -interactive=false \
        -kind Trunk \
        -gophercloud-client NewNetworkV2 \
        -gophercloud-module github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/trunks \
        -import-dependency Port \
        -import-dependency Project \
        -optional-create-dependency Project \
        -required-create-dependency Port

api/v1alpha1/trunk_type.go

mandre · 2025-12-01T15:59:05Z

api/v1alpha1/trunk_type.go

+	PortID string `json:"portID,omitempty"`
+
+	// segmentationType is the type of segmentation used.
+	// +kubebuilder:validation:MaxLength=1024


It's OK if it's higher in the Status than in the Spec (the opposite is not OK).

api/v1alpha1/trunk_type.go

github-actions · 2025-12-07T15:16:13Z

Failed to assess the semver bump. See logs for details.

winiciusallan

Hey @aldokimi, I've left more comments. This kind of PR tends to be very large, so I'll need to take another look. But for now, thanks for addressing the comments. =D

api/v1alpha1/trunk_type.go

winiciusallan · 2025-12-07T18:57:26Z

cmd/resource-generator/main.go

 	},
+	{
+		Name:             "Trunk",
+		ExistingOSClient: true,


Could you please remove this field? This uses an existing client, but we want to separate clients for each controller.

You may remove and run make generate. Maybe do you need to refactor other parts along the code.

But I don't really understand why removing this field is necessary, could elaborate more please?

Good question. To be honest, I don't know the exactly reason. I believe @mandre can explain it better.

My guess is, if you need some extra logic for a specific client, you won't impact the others implemented in the same interface. As I said, is just I guess, so don't take it as the truth 😅.

config/samples/openstack_v1alpha1_trunk.yaml

api/v1alpha1/trunk_type.go

internal/controllers/trunk/actuator.go

winiciusallan · 2025-12-08T20:33:32Z

internal/controllers/trunk/actuator.go

+		return nil
+	}
+
+	updateOpts.RevisionNumber = &osResource.RevisionNumber


May the revision number be nil, or does the API always returns a valid value?

internal/controllers/trunk/actuator.go

winiciusallan · 2025-12-08T20:42:05Z

internal/controllers/trunk/actuator.go

+			yield(nil, err)
+		}, nil
+	}
+	return func(yield func(*osResourceT, error) bool) {


Same about the comment on ListForAdoption.

internal/controllers/trunk/tests/trunk-create-minimal/00-assert.yaml

winiciusallan · 2025-12-19T17:18:06Z

@aldokimi I may be a little busy (resting...) in the next few days, so maybe I'll take a little while to review your commits, but at the start of the year, I should take a look into it.

Thanks for contributing, we really appreciate it. =D

winiciusallan · 2025-12-22T13:09:13Z

Hey @aldokimi, I had some time to take a look into your PR.

It looks like you have overrided your old implementation for Trunk types with the code generated by the scaffolding tool. Are you still working on this? Also, I saw that you changed a lot of tests files from other controllers, I believe we don't want to touch another controllers on this pull request. Do you have made this changes for a specific reason?

If you are still working on this, you can ignore my comment. Let me know when you believe it is ready for another review :)

aldokimi · 2025-12-22T15:46:59Z

Hey @aldokimi, I had some time to take a look into your PR.

It looks like you have overrided your old implementation for Trunk types with the code generated by the scaffolding tool. Are you still working on this? Also, I saw that you changed a lot of tests files from other controllers, I believe we don't want to touch another controllers on this pull request. Do you have made this changes for a specific reason?

If you are still working on this, you can ignore my comment. Let me know when you believe it is ready for another review :)

Hello @winiciusallan , I am still going to upload a last commit, the changes on the test files where because of the scaffolding tool, my last commit will be about fixing the issues regarding the scaffolding and then we will be ready for a new review!

Thank you for your comment, happy holidays!

aldokimi · 2025-12-28T03:32:32Z

Hello @winiciusallan and @mandre , happy new year!

So, I am sorry for this mess, this is the biggest PR of my life xD
I think now the PR is ready for a new review, if needed I am down for a code review session!

Thanks,

github-actions bot added the semver:major Breaking change label Nov 22, 2025

mandre mentioned this pull request Nov 24, 2025

Migrate from CAPO Internal OpenStackServer Controller to External ORC Controller kubernetes-sigs/cluster-api-provider-openstack#2814

Open

winiciusallan reviewed Nov 25, 2025

View reviewed changes

mandre reviewed Dec 1, 2025

View reviewed changes

aldokimi force-pushed the dev/trunk_type branch from ae0d632 to b1f5243 Compare December 7, 2025 15:15

github-actions bot removed the semver:major Breaking change label Dec 7, 2025

winiciusallan reviewed Dec 8, 2025

View reviewed changes

aldokimi added 4 commits December 17, 2025 13:53

Trunk resource: add Trunk resource controller implementation

e2ca1e1

Trunk resource: auto-generated files + fixing some bugs

29faf66

Trunk resource: increase test coverage + fix bugs

ba26726

Trunk resource: add improvments + fix bugs

8fe327a

winiciusallan mentioned this pull request Dec 18, 2025

📖 proposal: add support for defining trunk subports kubernetes-sigs/cluster-api-provider-openstack#2862

Open

Trunk resource: fixed testing

960bd52

aldokimi force-pushed the dev/trunk_type branch from b1f5243 to 960bd52 Compare December 18, 2025 23:48

github-actions bot added the semver:major Breaking change label Dec 18, 2025

Trunk type: fix scaffolding issues + minor bugs

545a5fa

aldokimi force-pushed the dev/trunk_type branch from 710a752 to 545a5fa Compare December 28, 2025 03:26

	PortRef KubernetesNameRef `json:"portRef"`
	PortRef KubernetesNameRef `json:"portRef,omitempty"`

Add Trunk resource controller implementation #581

Are you sure you want to change the base?

Add Trunk resource controller implementation #581

Conversation

aldokimi commented Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Key Features

Testing

E2E Testing

Checklist

Uh oh!

aldokimi commented Nov 22, 2025

Uh oh!

winiciusallan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mandre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 7, 2025

Uh oh!

winiciusallan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

winiciusallan commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

winiciusallan commented Dec 22, 2025

Uh oh!

aldokimi commented Dec 22, 2025

Uh oh!

aldokimi commented Dec 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

aldokimi commented Nov 22, 2025 •

edited

Loading

winiciusallan commented Dec 19, 2025 •

edited

Loading